Added sql code#196
Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because the script currently does not fulfill a core functional requirement: the order never gets an OrderItems row, so the created order is empty and will fail the tests. You correctly create an Orders record for CustomerID = 1 and update Products.WarehouseAmount inside a transaction, but the required item (ProductID = 1, Count = 1) is never inserted into OrderItems. This also means the key operation “add item to order and reduce stock” is not atomic, since only the stock update is transactional and the matching OrderItems insert is missing.
Critical issues to fix:
- After
INSERT INTO Orders (...) VALUES (...), capture the new order ID (e.g.,SET @orderId = LAST_INSERT_ID();). - Inside a single
START TRANSACTION/COMMITblock, both:INSERT INTO OrderItems (OrderID, ProductID, Count) VALUES (@orderId, 1, 1);UPDATE Products SET WarehouseAmount = WarehouseAmount - 1 WHERE ID = 1;
so they succeed or fail together.
Once you add the missing OrderItems insertion tied to the new OrderID and keep it in the same transaction as the inventory update, your solution will satisfy the business rules and should pass the tests. You already have the right idea with the transaction and inventory change; now it just needs to be completed by linking the order with its item correctly.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
|
|
||
| -- Some data should be created outside the transaction (here) | ||
|
|
||
| insert into Orders (CustomerID, Date) values (1, '2024-06-01'); |
There was a problem hiding this comment.
According to the description, the order must have one order item (AwersomeProduct, ID: 1, count: 1) stored in OrderItems. This script never inserts into OrderItems, so the created order remains empty and does not satisfy the task requirements.
| START TRANSACTION; | ||
|
|
||
| -- And some data should be created inside the transaction | ||
| update Products set WarehouseAmount = WarehouseAmount - 1 where ID = 1; |
There was a problem hiding this comment.
The product quantity update is inside a transaction, which is good, but it needs to be paired with an INSERT into OrderItems in the same transaction so that adding an item and reducing WarehouseAmount succeed or fail together.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there is still a critical correctness issue that breaks the core requirement of linking the order item to the newly created order. In task.sql, you currently hardcode OrderID = 1 in the INSERT INTO OrderItems statement (around the line where you insert ProductID = 1, Count = 1), which means the item may attach to an existing order with ID 1 instead of the order you just inserted. This will fail in environments where the new order’s ID is not 1, and it violates the requirement that “the SQL code creates a new order and adds a product to it” as a single, consistent operation.
To fix this, right after your INSERT INTO Orders (...) VALUES (...); statement, add something like SET @orderId = LAST_INSERT_ID(); to capture the ID of the row you just created. Then, update your INSERT INTO OrderItems inside the transaction to use @orderId instead of 1 for the OrderID column, and similarly update the related UPDATE Products (if it references OrderID) so everything is tied to that specific new order. Your use of a transaction around the OrderItems insert and the Products update is otherwise correct and matches the business rules, so once you link them with the captured @orderId, your script should fully satisfy the requirements. Keep going—you’re very close, and this adjustment will make your solution robust and correct across different database states.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
|
|
||
| -- And some data should be created inside the transaction | ||
|
|
||
| insert into OrderItems (OrderID, ProductID, Count) values (1, 1, 1); |
There was a problem hiding this comment.
Here you hardcode OrderID = 1. This breaks the requirement that the order item must belong to the newly created order. Capture the new order’s ID with LAST_INSERT_ID() after the INSERT INTO Orders and then use that variable (e.g., @orderId) here instead of 1.
| -- And some data should be created inside the transaction | ||
|
|
||
| insert into OrderItems (OrderID, ProductID, Count) values (1, 1, 1); | ||
| update Products set WarehouseAmount = WarehouseAmount - 1 where ID = 1; |
There was a problem hiding this comment.
This UPDATE is correctly inside the transaction with the OrderItems insert, but for full correctness it should be tied to the order you just created by using the captured @orderId above rather than assuming OrderID = 1 exists.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your solution is approved — the task.sql script correctly creates a new order for CustomerID = 1 with date 2023-01-01, then uses LAST_INSERT_ID() to link the order items to the newly created order. You correctly separate the non-transactional order creation from the transactional part where you insert the order item and update Products.WarehouseAmount, which matches the requirement that stock changes and item inserts happen together in a transaction. The use of USE ShopDB; and the correct table and column names shows you’re aligning with the expected database schema. Most remaining requirements (like recreating the database, ensuring tables are empty, and PR process) are procedural and not expected to appear directly in this script.
If you want to improve this further, you could consider wrapping the order-item creation and stock update logic into a reusable stored procedure, but this is not required for the task. Overall, you’ve implemented the core logic cleanly and in line with the checklist, and you’re ready to move on. Good work applying transactions and LAST_INSERT_ID() correctly — these are important concepts to master for real-world database operations.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
No description provided.